fix(dashboard): resolve dropdown popup positioning#36963
fix(dashboard): resolve dropdown popup positioning#36963msyavuz merged 1 commit intoapache:masterfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Code Review Agent Run #6ec00e
Actionable Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts - 1
- Deprecated API Usage · Line 36-40
Review Details
-
Files reviewed - 2 · Commit Range:
3f38a44..3f38a44- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts
-
Files skipped - 0
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts
Outdated
Show resolved
Hide resolved
6e73e3e to
a164e50
Compare
Code Review Agent Run #ba1917Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
fd1b92d to
05bbab3
Compare
05bbab3 to
4d73f0e
Compare
Code Review Agent Run #653b80Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts
Outdated
Show resolved
Hide resolved
|
For people who are going to QA this. The Select is used basically everywhere, so we might need to do a sanity check in most pages, including Modals and Popovers. |
superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts
Outdated
Show resolved
Hide resolved
4d73f0e to
81a9b68
Compare
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
superset-frontend/packages/superset-ui-core/src/components/Select/types.ts
Outdated
Show resolved
Hide resolved
81a9b68 to
109b61c
Compare
|
🎪 Showtime deployed environment on GHA for 82389c6 • Environment: http://35.92.213.236:8080 (admin/admin) |
There was a problem hiding this comment.
Code Review Agent Run #6c57cd
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/SaveModal.tsx - 1
- Potential undefined access · Line 541-547
Additional Suggestions - 5
-
superset-frontend/src/explore/components/SaveModal.tsx - 2
-
Incorrect URL hash for 'Out of tab' · Line 323-325When 'Out of tab' is selected, the URL incorrectly appends '#OUT_OF_TAB', which doesn't correspond to a real tab. The condition should check that selectedTab.value exists and isn't 'OUT_OF_TAB' to avoid invalid navigation hashes.
Code suggestion
@@ -323,1 +323,1 @@ - if (this.state.selectedTab?.value) { + if (this.state.selectedTab?.value && this.state.selectedTab.value !== 'OUT_OF_TAB') {
-
State inconsistency in error handling · Line 500-500When the tabs API fails or returns invalid data, selectedTab remains unchanged, potentially showing a stale selection. Reset it to undefined in error cases for consistent UI state.
-
-
tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py - 1
-
Test Logic Duplication · Line 27-153These tests duplicate the fallback logic from `get_chart_data` instead of testing the implementation. This approach won't catch bugs in the actual code since it reimplements the same logic. Consider refactoring to extract the metric/groupby logic into a helper and test the real function with proper mocks.
-
-
superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx - 1
-
Inaccurate test description · Line 69-69The test name claims to verify 'all known HTML tags' but only tests 15 out of 58 tags in KNOWN_HTML_TAGS. This could mislead readers about test coverage. Consider renaming to 'various' or expanding to cover more/all tags if comprehensive testing is intended.
Code suggestion
@@ -69,1 +69,1 @@ - it('should return true for all known HTML tags', () => { + it('should return true for various known HTML tags', () => {
-
-
superset/mcp_service/auth.py - 1
-
Function too complex: exceeds statement limit · Line 175-175The `mcp_auth_hook` function has 58 statements, exceeding the limit of 50. Consider extracting helper functions to reduce complexity, such as moving signature manipulation logic into a separate function.
Code suggestion
@@ -172,6 +172,28 @@ logger.warning("Error in finally block: %s", e) +def _setup_wrapper_signature(wrapper: Any, tool_func: F) -> None: + """Set up the wrapper function's signature, removing ctx parameter.""" + import inspect + from fastmcp import Context as FMContext + + tool_sig = inspect.signature(tool_func) + has_var_positional = any( + p.kind == inspect.Parameter.VAR_POSITIONAL for p in tool_sig.parameters.values() + ) + + if not has_var_positional: + new_params = [] + for _name, param in tool_sig.parameters.items(): + if param.annotation is FMContext or ( + hasattr(param.annotation, "__name__") + and param.annotation.__name__ == "Context" + ): + continue + new_params.append(param) + wrapper.__signature__ = tool_sig.replace(parameters=new_params) + if "ctx" in wrapper.__annotations__: + del wrapper.__annotations__["ctx"] + + def mcp_auth_hook(tool_func: F) -> F: # noqa: C901 """ Authentication and authorization decorator for MCP tools. @@ -280,28 +302,7 @@ # Copy docstring from original function (not wrapper, which may have lost it) new_wrapper.__doc__ = tool_func.__doc__ - # Set __signature__ from the original function, but: - # 1. Remove ctx parameter - FastMCP tools don't expose it to clients - # 2. Skip if original has *args (parse_request output has its own handling) - from fastmcp import Context as FMContext - - tool_sig = inspect.signature(tool_func) - has_var_positional = any( - p.kind == inspect.Parameter.VAR_POSITIONAL for p in tool_sig.parameters.values() - ) - - if not has_var_positional: - # For functions without *args, preserve signature but remove ctx - new_params = [] - for _name, param in tool_sig.parameters.items(): - # Skip ctx parameter - FastMCP tools don't expose it to clients - if param.annotation is FMContext or ( - hasattr(param.annotation, "__name__") - and param.annotation.__name__ == "Context" - ): - continue - new_params.append(param) - new_wrapper.__signature__ = tool_sig.replace( # type: ignore[attr-defined] - parameters=new_params - ) - - # Also remove ctx from annotations to match signature - if "ctx" in new_wrapper.__annotations__: - del new_wrapper.__annotations__["ctx"] - # For functions with *args (parse_request output), the signature - # is already set by parse_request without ctx. + _setup_wrapper_signature(new_wrapper, tool_func) return new_wrapper # type: ignore[return-value]
-
Review Details
-
Files reviewed - 21 · Commit Range:
e4ffd4f..82389c6- docs/yarn.lock
- superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx
- superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx
- superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx
- superset-frontend/packages/superset-ui-core/src/components/Select/constants.ts
- superset-frontend/packages/superset-ui-core/src/components/Select/types.ts
- superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
- superset-frontend/packages/superset-ui-core/src/utils/html.tsx
- superset-frontend/src/dashboard/constants.ts
- superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx
- superset-frontend/src/explore/components/ControlPanelsContainer.tsx
- superset-frontend/src/explore/components/SaveModal.test.jsx
- superset-frontend/src/explore/components/SaveModal.tsx
- superset-frontend/src/explore/types.ts
- superset/mcp_service/auth.py
- superset/mcp_service/chart/tool/get_chart_data.py
- superset/mcp_service/chart/validation/runtime/__init__.py
- superset/templates/superset/spa.html
- tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py
- tests/unit_tests/mcp_service/chart/validation/__init__.py
- tests/unit_tests/mcp_service/chart/validation/test_runtime_validator.py
-
Files skipped - 7
- docs/package.json - Reason: Filter setting
- superset-frontend/package-lock.json - Reason: Filter setting
- superset-frontend/package.json - Reason: Filter setting
- superset-frontend/packages/superset-core/package.json - Reason: Filter setting
- superset-frontend/packages/superset-ui-demo/package.json - Reason: Filter setting
- superset-websocket/package-lock.json - Reason: Filter setting
- superset-websocket/package.json - Reason: Filter setting
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
| } else { | ||
| const firstTab = treeData[0]; | ||
| this.setState({ | ||
| tabsData: treeData, | ||
| selectedTab: { value: firstTab.value, label: firstTab.title }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Accessing treeData[0] without checking if treeData is empty can lead to undefined errors when no tabs exist.
Code suggestion
Check the AI-generated fix before applying
| } else { | |
| const firstTab = treeData[0]; | |
| this.setState({ | |
| tabsData: treeData, | |
| selectedTab: { value: firstTab.value, label: firstTab.title }, | |
| }); | |
| } | |
| } else { | |
| if (treeData.length > 0) { | |
| const firstTab = treeData[0]; | |
| this.setState({ | |
| tabsData: treeData, | |
| selectedTab: { value: firstTab.value, label: firstTab.title }, | |
| }); | |
| } | |
| } |
Citations
- Rule Violated: dev-standard.mdc:16
Code Review Run #6c57cd
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
|
Hi @reynoldmorel it looks like you may need to rebase your branch before we can review. Thanks! |
addressed |
82389c6 to
7822c22
Compare
Code Review Agent Run #13bf58Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
(cherry picked from commit 43653d1)
(cherry picked from commit 43653d1)
SUMMARY
When zooming in any page (150% to 200% zoom), the dropdown filter popup was positioning over the dropdown input creating a bad user experience when manipulating the dropdown component.
The fix simply aligns the dropdown popup always below the dropdown input.
This fix will affect all the places that are using this dropdown component.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE:
The dropdown covers the input box, preventing any filtering or selection via typing. This breaks basic usability on smaller screens.
AFTER
short_video_after.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION